Split LDAP user filters #996
Conversation
butonic
force-pushed
the
split-ldap-filters
branch
2 times, most recently
from
86c9db2 to
6f4e803
Compare
pkg/user/manager/ldap/ldap.go
Outdated
| fmt.Sprintf(m.userfilter, uid.OpaqueId), // TODO this is screaming for errors if filter contains >1 %s | ||
| []string{m.schema.DN, m.schema.UID, m.schema.Mail, m.schema.DisplayName}, | ||
| m.getUserFilter(uid), | ||
| []string{m.schema.DN, m.schema.CN, m.schema.UID, m.schema.Mail, m.schema.DisplayName}, |
There was a problem hiding this comment.
we add the cn attribute so we can return a user with an id that is different from the username
pkg/user/manager/ldap/ldap.go
Outdated
| @@ -154,7 +180,7 @@ func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User | |||
| } | |||
| u := &userpb.User{ | |||
| Id: id, | |||
| Username: sr.Entries[0].GetAttributeValue(m.schema.UID), | |||
| Username: sr.Entries[0].GetAttributeValue(m.schema.CN), | |||
There was a problem hiding this comment.
here we use the previously fetched cn as the username. they can be configured to the same attribute and the code will work as before
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
butonic
force-pushed
the
split-ldap-filters
branch
from
6f4e803 to
ed64c21
Compare
| @@ -244,13 +270,17 @@ func (m *manager) GetUserGroups(ctx context.Context, uid *userpb.UserId) ([]stri | |||
| groups := []string{} | |||
|
|
|||
| for _, entry := range sr.Entries { | |||
| // FIXME this makes the users groups use the cn, not an immutable id | |||
| // FIXME 1. use the memberof or members attribute of a user to get the groups | |||
| // FIXME 2. ook up the id for each group | |||
There was a problem hiding this comment.
this needs to be cleaned up to actually fetch groups. AFAIU it will require the memberof overlay. In any case the member attribute needs to be made configurable as well. As part of a subsequent PR.
There was a problem hiding this comment.
Looks good to me. We also need to make the corresponding change to the auth/ldap package. There we still use the UID attribute as username.
Also, can we just make config an object of the manager struct? The fields are just repeated so that's a bit off-putting.
One question for my understanding. The m.schema.UID attribute would have the same value as uid.OpaqueId, right?
|
@ishank011 good catch! The I think we need to introduce a new api call to properly handle that though, tracked in #998 |
butonic
force-pushed
the
split-ldap-filters
branch
from
ef21c69 to
db04e95
Compare
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
butonic
force-pushed
the
split-ldap-filters
branch
from
db04e95 to
66a7e9d
Compare
The current LDAP user filters only allow a single
%sto be replaced with the relevant string. I moved to filter templates that can use the CS3 user id properties{{.OpaqueId}}and{{.Idp}}. Furthermore,I introduced a new find filter that is used when searching for users. An example config would be
As you can see the userfilter is used to lookup a single user, whereas the findfilter is for a broader search, e.g. used when searching for share recipients.
Related: #326